Skip to content

optionally inject the wallet public key as a header#21

Open
benarena wants to merge 1 commit intoFigureTechnologies:mainfrom
benarena:public-key-header
Open

optionally inject the wallet public key as a header#21
benarena wants to merge 1 commit intoFigureTechnologies:mainfrom
benarena:public-key-header

Conversation

@benarena
Copy link
Copy Markdown
Contributor

@benarena benarena requested review from jphorec and mtps as code owners June 30, 2022 16:39
@mtps
Copy link
Copy Markdown
Collaborator

mtps commented Jul 1, 2022

@benarena is there any payload claims data that shouldn't be in the headers? I'm wondering if at this point it would be easier to provide a blanket claims -> x-wallet-* mapping of sorts and do away with all the custom config for each field (maybe instead providing a config for a "header prefix" defaulting to "x-wallet-* as above mentioned).

@benarena
Copy link
Copy Markdown
Contributor Author

benarena commented Jul 1, 2022

@benarena is there any payload claims data that shouldn't be in the headers? I'm wondering if at this point it would be easier to provide a blanket claims -> x-wallet-* mapping of sorts and do away with all the custom config for each field (maybe instead providing a config for a "header prefix" defaulting to "x-wallet-* as above mentioned).

The JWT is not encrypted, so anything in the JWT can be safely added to the headers without fear of introducing a leak. Some claims are of questionable value (issuer, iat, exp), but certainly no harm in inclusion so long as the header name is comprehensible. x-wallet-iat may be confusing to the consumer.

Additionally, I'm happy to set a default header name rather than default to non-inclusion here if there is a preference for that. I've heard you'd like to split out the RBAC piece into its own plugin so it makes sense to make opinionated decisions related only to the verification and header inclusion in this plugin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants